Expose standard Rust traits on FFI error types#1485
Expose standard Rust traits on FFI error types#1485spacebear21 merged 2 commits intopayjoin:masterfrom
Conversation
Coverage Report for CI Build 24527851470Coverage remained the same at 84.38%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
The dart error here looks unrelated to the changes in this PR, @chavic can you confirm? |
|
Reviewed the changes. Exposing Display, Error, and Debug on FFI error types is the right move, this makes them usable with anyhow and ? in Rust glue code, and improves error messages for FFI consumers. One small question: the PR description mentions fixing "Instance of *Error" strings. Is this primarily improving the Dart/Kotlin/Swift binding experience? Might be helpful to note in the commit body. Regarding the unrelated Dart CI failure, agreed it's pre-existing and not related to this change. |
This seems to relate to the errors being turned to "Exception" like in the early stage of #1428 (comment) |
spacebear21
force-pushed
the
ffi-expose-standard-traits
branch
from
07df5e4 to
54e06c7
Compare
Yep, confirmed that updating uniffi-dart to the latest git rev fixes the issue and added this in a9e0cab. |
spacebear21
force-pushed
the
ffi-expose-standard-traits
branch
from
54e06c7 to
4d2855c
Compare
Uniffi-Dart/uniffi-dart#120 fixed an issue where uniffi-dart substitutes `Error` with `Exception` but didn't correctly reference the updated name in method signatures, etc.
This fixes the string representation of the FFI errors to use the Rust Display string instead of an opaque `Instance of *Error` message in downstream language bindings.
There was a problem hiding this comment.
TACK 4d2855c
I added some dummy Display tests to visualize the errors in python with the following tests.
Python Display Tests
class TestErrorDisplay(unittest.TestCase):
def test_address_parse_error_display(self):
try:
payjoin.ReceiverBuilder(
"not-an-address",
"https://example.com",
payjoin.OhttpKeys.decode(bytes.fromhex(
"01001604ba48c49c3d4a92a3ad00ecc63a024da10ced02180c73ec12d8a7ad2cc91bb483824fe2bee8d28bfe2eb2fc6453bc4d31cd851e8a6540e86c5382af588d370957000400010003"
)),
)
self.fail("expected error")
except payjoin.ReceiverBuilderError as e:
msg = str(e)
self.assertNotIn("Instance of", msg)
self.assertTrue(len(msg) > 0)
def test_sender_input_error_display(self):
uri = payjoin.Uri.parse(
"bitcoin:tb1q6d3a2w975yny0asuvd9a67ner4nks58ff0q8g4?pj=https://example.com/pj"
).check_pj_supported()
try:
payjoin.SenderBuilder("not-a-psbt", uri)
self.fail("expected error")
except payjoin.SenderInputError as e:
self.assertNotIn("Instance of", str(e))
def test_pj_parse_error_display(self):
try:
payjoin.Uri.parse("not-a-uri")
self.fail("expected error")
except payjoin.PjParseError as e:
print(f"\ntype={type(e).__name__}")
print(f"{str(e)!r}")
print(f"{repr(e)!r}")
self.assertIn("Error parsing the payjoin URI", str(e)) Saw this response.
$ uv run python -m unittest test.test_payjoin_unit_test.TestErrorDisplay.test_pj_parse_error_display -v test_pj_parse_error_display (test.test_payjoin_unit_test.TestErrorDisplay.test_pj_parse_error_display) ...
type=PjParseError
'Error parsing the payjoin URI: invalid BIP21 URI'
'PjParseError { msg: "invalid BIP21 URI" }'
okNot right now but I think it would be really nice to get some error Display test coverage in payjoin soon maybe as a v1.1 milestone
caarloshenriq
left a comment
caarloshenriq
left a comment
There was a problem hiding this comment.
tACK 4d2855c
Ran the Python Display tests from @benalleng locally after regenerating the bindings:
test_address_parse_error_display ... ok
test_pj_parse_error_display ...
type=PjParseError
'Error parsing the payjoin URI: invalid BIP21 URI'
'PjParseError { msg: "invalid BIP21 URI" }'
ok
test_sender_input_error_display ... ok
Agreed, we should also start thinking about a strategy for cross-testing these things across the all downstream language bindings. Maybe we can pull out test inputs and expected results into shared test vectors to loop over in each language's test module, or something like that. |
5 participants
This fixes the string representation of the FFI errors to use the Rust Display string instead of an opaque
Instance of *Errormessage. See #1264 (comment) for more context.Discovered and authored with Claude.
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.